Skip to content

Allow tests to use different mpas_analysis environment#763

Merged
forsyth2 merged 1 commit intomainfrom
add-mpas-cmds
Dec 17, 2025
Merged

Allow tests to use different mpas_analysis environment#763
forsyth2 merged 1 commit intomainfrom
add-mpas-cmds

Conversation

@forsyth2
Copy link
Copy Markdown
Collaborator

@forsyth2 forsyth2 commented Dec 2, 2025

Summary

Objectives:

  • Add the ability to specify environment_commands for mpas_analysis in testing. (Note this was always possible as a user: just set environment_commands in the cfg).

Issue resolution:

Select one: This pull request is...

  • a bug fix: increment the patch version
  • a small improvement: increment the minor version
  • a new feature: increment the minor version
  • an incompatible (non-backwards compatible) API change: increment the major version

Small Change

  • To merge, I will use "Squash and merge". That is, this change should be a single commit.
  • Logic: I have visually inspected the entire pull request myself.
  • Pre-commit checks: All the pre-commits checks have passed.

@forsyth2 forsyth2 self-assigned this Dec 2, 2025
@forsyth2 forsyth2 added the Testing Files in `tests` modified label Dec 2, 2025
forsyth2
forsyth2 commented Dec 2, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chengzhuzhang I think this should be fine to merge.

A couple notes:

  • The bundle cfgs weren't updated because they don't have a mpas_analysis task set up.
  • I did update the legacy cfgs, which we normally think of as "frozen", but the change is the value of environment_commands, not the parameters themselves. So, it's really no different than, for example, running the latest e3sm_diags dev environment on a legacy cfg.

@forsyth2 forsyth2 mentioned this pull request Dec 2, 2025
Merged
7 tasks
@chengzhuzhang
Copy link
Copy Markdown
Collaborator

chengzhuzhang commented Dec 2, 2025

okay, I thought specifying environment_commands was already enabled. This PR is just for testing. If it is the case, perhaps to update the title to avoid confusion.

@forsyth2 forsyth2 changed the title Add mpas_analysis_environment_commands Allow tests to use different mpas_analysis environment Dec 2, 2025
@forsyth2
Copy link
Copy Markdown
Collaborator Author

forsyth2 commented Dec 2, 2025

I thought specifying environment_commands was already enabled.

Yes, sorry, I tried to clear that up in the PR description. This is exclusively for auto-generating the test cfgs.

perhaps to update the title to avoid confusion.

Good idea; I've updated it to "Allow tests to use different mpas_analysis environment".

@forsyth2
Copy link
Copy Markdown
Collaborator Author

forsyth2 commented Dec 2, 2025

These are sort-of meta-environment commands -- i.e., <task_name>_environment_commands is really saying "what should we set as environment_commands in each of the auto-generated test cfgs for task <task_name>?"

@forsyth2
Copy link
Copy Markdown
Collaborator Author

forsyth2 commented Dec 2, 2025

That's also why nothing changes in default.ini, which is where a user-facing parameter change would go.

@forsyth2
Copy link
Copy Markdown
Collaborator Author

forsyth2 commented Dec 2, 2025 •
edited
Loading

I'm going to hold off on merging this until we get a passing test of #760. Based on #760 (comment), it makes sense to add a conditional change to the mapMpiTasks used in testing based on the environment chosen.

@forsyth2
Copy link
Copy Markdown
Collaborator Author

forsyth2 commented Dec 17, 2025

Now that #760 has merged, I'm also going to merge this testing fix.

@forsyth2 forsyth2 merged commit 9d986de into main Dec 17, 2025
6 checks passed
@forsyth2 forsyth2 deleted the add-mpas-cmds branch December 17, 2025 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@forsyth2 forsyth2

Labels

Testing Files in `tests` modified

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@forsyth2 @chengzhuzhang